Skip to content

Conversation

mleczakm
Copy link
Contributor

@mleczakm mleczakm commented Aug 5, 2025

…ling by default messenger way

Fixes #205

@mleczakm mleczakm requested a review from a team as a code owner August 5, 2025 19:33
@Rastusik
Copy link
Member

Rastusik commented Aug 6, 2025

heye @mleczakm ! thank you for the contribution!

I'm afraid I'm not able to merge this without a reproducer test.

But there is an important design question here - is it really needed to support retries in the task workers? The thing is that we're in non-persistent context here (php process memory). Retries usually use some kind of a durable queue. With too many retries in Swoole, I'm afraid there would be a risk for out of memory errors.

Maybe you should reconsider your design decisions?

@mleczakm
Copy link
Contributor Author

mleczakm commented Aug 6, 2025

@Rastusik That exception logging is also silencing errors. It breaks error handling in Symfony and interferes with message processing in Messenger. These are not my design decisions, but part of the Symfony Messenger design. Since this is a bridge for it, it should fully support that behavior.

What’s also important – it breaks the failed queue mechanism. While it can be fixed by reconfiguring services (so it's possible to fix it in userland), there’s no point in breaking the default framework behavior with no real profit - exception would be logged anyway.

@Rastusik
Copy link
Member

Rastusik commented Aug 6, 2025

I believe the exception catching is for a reason there (need to find time to look into it closer). if the exception won't be caught, the swoole worker process will terminate.
the point of this messenger integration is just to use the swoole task mechanism with a common symfony messenger interface.

I can look later where the deleted class is exactly hooked in, but I'm still not sure if I am the one who's missing context, or you, or us both :)

@Rastusik Rastusik merged commit ea04d6c into symfony-swoole:develop Aug 14, 2025
4 of 5 checks passed
@Rastusik
Copy link
Member

@Rastusik That exception logging is also silencing errors. It breaks error handling in Symfony and interferes with message processing in Messenger. These are not my design decisions, but part of the Symfony Messenger design. Since this is a bridge for it, it should fully support that behavior.

@mleczakm can you elaborate more on this please? Which part is silencing errors? what happens then? Does the failed mechanism even start on problems right now? The transport implementation is relatively weird, the sender just sends to swoole task and the task process just loads the from sender, stored in swoole, and sends it to the probably sync messeger instance. Afaik error handling in messenger is implemented by a normal worker, isn't it? The current implementation seems to be skipping handling and it's calling the an instance of MessageInterface. So probably this is showing that there are no retries configured in this implementation, because proper symfony events are not triggered.

Sorry I accidentally merged this PR, then I needed to reset it. If you can give m a reproducer of what you are trying to achieve and show me that it works, we can think about a solution here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Retry and failed transport not working for worker tasks
2 participants